-
Notifications
You must be signed in to change notification settings - Fork 2k
feat(core): Use ngStrictDi in production #1302
Conversation
Slightly changed layout template to use Angular's Strict Dependency Injection if the app is running in production. Fixes part of #1294
@@ -1,5 +1,5 @@ | |||
<!DOCTYPE html> | |||
<html lang="en"> | |||
<html lang="en" {% if env === "production" %}ng-strict-di{% endif %}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see a downside to not having this turned on all the time. Is there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ilanbiala Wouldn't work if you use ngAnnotate? (I saw a PR at some point for that but not sure if it's already included)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ilanbiala right now ngAnnotate is only being used in build task for production so I kept ngStrictDi only for production otherwise it will break.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could just annotate in development too, which I'm not opposed to. Honestly I think ngStrictDi would prevent errors in production if we also check it in development.
@meanjs/contributors any of you care to take it further to implement @ilanbiala suggested changes with making it work regardless of the environment? it's a rather small change. |
@pgrodrigues you're also welcome to just update the PR to accommodate the changes. |
@codydaig is already taking care of this. Time to celebrate the Euro2016 win :) |
@pgrodrigues definitely! :) |
Slightly changed layout template to use Angular's Strict
Dependency Injection if the app is running in production.
Fixes part of #1294